create vaults and auctions release (proposal 76) #172

Chris-Hibbert · 2024-09-04T17:13:58Z

closes: #171

Description

add tests and assets for the vaults and auctions release agoric-upgrade-16av.

dckc · 2024-09-06T13:43:30Z

For reference, this PR represents the following mainnet governance decision:

#76. Replace Auctioneer and Upgrade Vaults
Voting Time : Sep 1st 2024, 14:47:11 ~ Sep 4th 2024, 14:47:11

I presume it's ok to tweak the PR title to add (proposal 76).

dckc

much of what I comment on pre-dates this PR, but let's please discuss relying on alleged names for correctness.

I didn't get all the way thru, but I've got a meeting coming up, so I'm sharing this much.

dckc · 2024-09-06T13:54:21Z

proposals/76:vaults-auctions/upgradeVaults.test.js

+const verifyVaultPriceUpdate = async t => {
+  const quote = await getVaultPrices(0);
+
+  t.true(quote.value[0].amountIn.brand.includes(' ATOM '));


the brand is a string? Is that a result of a name service lookup or just an alleged/debug label? I'll have to check...

I checked. It's an alleged name. something like $4.Alleged ATOM Brand.

Options include:

let it slide. I've done too much of that lately.

add a comment telling other folks to "do as I say, not as I do" about reliability of alleged names

fix it by not throwing the boardIds that serve as clist identities.

... others?

Let's discuss.

Is your concern that the brand may have been forged somewhere in the history of A3P?

My concern is that we're testing something that's not part of the API for brands.

Also, this is fragile: in due course, there could be other alleged names that include the same string.

Those would occur after this in history so can't affect this particular test.

I'm okay with #2. A3P is a controlled environment in which there are no adversaries submitting faked names.

We discussed this at the Zoe meeting. The issue here is more about comparing to strings extracted from vstorage without unmarshalling the capData. It seems straightforward for a3p tests to get unmarshalled data. @dckc is investigating.

dckc · 2024-09-06T14:38:38Z

proposals/76:vaults-auctions/submission/add-auction-permit.json

+      "runModuleBehaviors": "makeCoreProposalBehavior"
+    }
+  }
+}


This missing newline matches what we built in ci, but I'm not sure it matches what's on chain. See #175

dckc · 2024-09-06T14:39:06Z

proposals/76:vaults-auctions/submission/upgrade-vaults-permit.json

+      "runModuleBehaviors": "makeCoreProposalBehavior"
+    }
+  }
+}


likewise this missing newline

packages/synthetic-chain/package.json

dckc · 2024-09-06T14:42:45Z

proposals/76:vaults-auctions/README.md

+The `assets` for this proposal were generated as part of [creating
+the release](https://github.com/Agoric/agoric-sdk/releases/tag/agoric-upgrade-16av).


That might be a useful cross reference, but it's incidental at this point. What's critical is that the assets here match what's in mainnet proposal 76.

dckc · 2024-09-06T15:09:33Z

proposals/76:vaults-auctions/upgradeVaults.test.js

+  const atomOutPre = await getPriceQuote('ATOM');
+  t.is(atomOutPre, '+12010000');


getPriceQuote returns unparsed smallcaps, with object references disconnected from their boardIds. This tech debt is growing and growing. Here we compare what's actually an integer to its smallcaps string representation. At least that doesn't throw away info like object references.

I suppose it pre-dates this PR, so separate issue:

getPriceQuote exposes smallcaps details; loses object identities #176

dckc · 2024-09-06T15:15:30Z

proposals/76:vaults-auctions/upgradeVaults.test.js

+  const liveOffer = await getLiveOffers(USER1ADDR);
+  t.true(liveOffer[0].includes(BID_OFFER_ID));


Suggested change

const liveOffer = await getLiveOffers(USER1ADDR);

t.true(liveOffer[0].includes(BID_OFFER_ID));

const liveOffers = await getLiveOffers(USER1ADDR);

t.true(liveOffers[0].includes(BID_OFFER_ID));

and why check .includes(BID_OFFER_ID) rather than === BID_OFFER_ID? Does getLiveOffers not return an array of offer ids?

I don't think it does. I think it's a larger structure I didn't know how to reliably untangle.

dckc · 2024-09-06T15:21:31Z

proposals/76:vaults-auctions/upgradeVaults.test.js

+  t.log('make new auction');
+  await checkAuctionVat(t);


Are we actually making a new auction vat here? this whole test runs after the upgrade, yes? Here we might confirm that the auctioneer is new, but we're not making it, right?

The new auction vat already exists before the test starts.

dckc · 2024-09-06T15:24:44Z

proposals/76:vaults-auctions/vatDetails.js

+// TODO(cth) get from synthetic-chain
+/** @param {string} vatName */
+export const getDetailsMatchingVats = async vatName => {
+  const fullPath = swingstorePath.replace(/^~/, NonNullish(HOME));


synthetic chain exports a HOME that might be null? strange.

elsewhere I see env.HOME. Why not use env.HOME here as well?

dckc · 2024-09-06T15:52:40Z

proposals/76:vaults-auctions/vatDetails.js

+export const getDetailsMatchingVats = async vatName => {
+  const fullPath = swingstorePath.replace(/^~/, NonNullish(HOME));
+
+  const db = dbOpenAmbient(fullPath, { readonly: true });


The db should be passed into getDetailsMatchingVats per OCap Discipline. Again, pre-dates this PR, so separate issue:

transitional approach to OCap Discipline agoric-sdk#10038

This copy of getDetailsMatchingVats is here until there's a version published from synthetic-chain.

Please suggest revisions in #174.

dckc

Good work

packages/synthetic-chain/public/upgrade-test-scripts/run_eval.sh

turadg · 2024-09-09T20:19:15Z

proposals/76:vaults-auctions/package.json

+{
+  "agoricProposal": {
+    "type": "/agoric.swingset.CoreEvalProposal",
+    "releaseNotes": "https://github.com/Agoric/agoric-sdk/releases/tag/agoric-upgrade-16av",


the build-proposals job is failing because this trailing comma is invalid JSON.

maybe we should add a "lint" job? it could report if yarn doctor reported or changed anything and also do eslint style linting on files.

* chore: automate fetching parts of CoreEval proposal 76 * chore: save complete transaction for proposal 76 * chore: tweak whitespace to match on-chain data * fixup! chore: automate fetching parts of CoreEval proposal 76

refs: #9928 ## Description Clean up in a3p-integration, as a follow-up to the vaults&auctions release on mainNet, and [addition of that release to `agoric-3-proposals`](Agoric/agoric-3-proposals#172). ### Security Considerations N/A ### Scaling Considerations N/A ### Documentation Considerations I will add more details of this process to CONTRIBUTING in a-3-p. ### Testing Considerations keeping things clean so testing can continue. ### Upgrade Considerations upgrade and iterate!

Chris-Hibbert added enhancement New feature or request test labels Sep 4, 2024

Chris-Hibbert self-assigned this Sep 4, 2024

Chris-Hibbert force-pushed the 171-create-VARelease branch 7 times, most recently from 34d84ee to 656e1db Compare September 5, 2024 23:17

Chris-Hibbert requested a review from dckc September 6, 2024 00:05

Chris-Hibbert mentioned this pull request Sep 6, 2024

add the ability to find details for multiple vats #174

Merged

dckc changed the title ~~create vaults and auctions release~~ create vaults and auctions release (proposal 76) Sep 6, 2024

This was referenced Sep 6, 2024

fetch proposal 76 details from the chain #175

Merged

getPriceQuote exposes smallcaps details; loses object identities #176

Open

dckc reviewed Sep 6, 2024

View reviewed changes

dckc approved these changes Sep 7, 2024

View reviewed changes

turadg requested changes Sep 7, 2024

View reviewed changes

packages/synthetic-chain/public/upgrade-test-scripts/run_eval.sh Outdated Show resolved Hide resolved

Chris-Hibbert requested a review from turadg September 9, 2024 18:42

turadg approved these changes Sep 9, 2024

View reviewed changes

turadg reviewed Sep 9, 2024

View reviewed changes

Chris-Hibbert and others added 2 commits September 9, 2024 13:47

test: add 76:vaults-auctions

66e965d

fetch proposal 76 details from the chain (#175)

eaf15eb

* chore: automate fetching parts of CoreEval proposal 76 * chore: save complete transaction for proposal 76 * chore: tweak whitespace to match on-chain data * fixup! chore: automate fetching parts of CoreEval proposal 76

Chris-Hibbert force-pushed the 171-create-VARelease branch from cc1fc8a to eaf15eb Compare September 9, 2024 20:48

Chris-Hibbert merged commit 01f675b into main Sep 10, 2024
2 checks passed

Chris-Hibbert deleted the 171-create-VARelease branch September 10, 2024 00:03

Chris-Hibbert mentioned this pull request Sep 10, 2024

test: drop a:vaults-auctions/ from a3p-integration Agoric/agoric-sdk#10053

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

create vaults and auctions release (proposal 76) #172

create vaults and auctions release (proposal 76) #172

Chris-Hibbert commented Sep 4, 2024 •

edited

Loading

dckc commented Sep 6, 2024

dckc left a comment

dckc Sep 6, 2024

dckc Sep 6, 2024

turadg Sep 6, 2024

dckc Sep 6, 2024 •

edited

Loading

turadg Sep 6, 2024

Chris-Hibbert Sep 6, 2024

Chris-Hibbert Sep 11, 2024

dckc Sep 6, 2024

Chris-Hibbert Sep 6, 2024

dckc Sep 6, 2024

Chris-Hibbert Sep 6, 2024

dckc Sep 6, 2024

Chris-Hibbert Sep 6, 2024

dckc Sep 6, 2024

dckc Sep 6, 2024

Chris-Hibbert Sep 6, 2024

dckc Sep 6, 2024

Chris-Hibbert Sep 6, 2024

dckc Sep 6, 2024

dckc Sep 6, 2024

Chris-Hibbert Sep 6, 2024

dckc Sep 6, 2024

Chris-Hibbert Sep 6, 2024

dckc left a comment

turadg Sep 9, 2024

		The `assets` for this proposal were generated as part of [creating
		the release](https://github.com/Agoric/agoric-sdk/releases/tag/agoric-upgrade-16av).

		const atomOutPre = await getPriceQuote('ATOM');
		t.is(atomOutPre, '+12010000');

		const liveOffer = await getLiveOffers(USER1ADDR);
		t.true(liveOffer[0].includes(BID_OFFER_ID));

create vaults and auctions release (proposal 76) #172

create vaults and auctions release (proposal 76) #172

Conversation

Chris-Hibbert commented Sep 4, 2024 • edited Loading

Description

dckc commented Sep 6, 2024

dckc left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dckc Sep 6, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dckc left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Chris-Hibbert commented Sep 4, 2024 •

edited

Loading

dckc Sep 6, 2024 •

edited

Loading